Skip to content

✨ Add set and delete_ to native bytes/string in storage #1371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

shuhuiluo
Copy link
Contributor

Description

Add set and delete_ functions to simplify handling of bytes and string storage references in LibBytes and LibString. These changes enable efficient and memory-safe operations when assigning or deleting storage references directly.

Rationale

As the following example shows, Solidity only allows assignments to state variables or to members of local variables of storage struct type. One cannot assign a bytes array in memory/calldata to a local bytes array storage reference. More often than not, people first read from a member of an array/mapping state variable, dataArray[i], and then write to the same member using dataArray[i] = x, which involves two slot calculations and therefore two keccak256s.

A workaround is to cast the bytes storage reference to a wrapper struct, which isn't obvious and clean. Besides, the bytecode for bytes assignment and deletion generated by solc is suboptimal.

While Solady already has BytesStorage and StringStorage, switching to a different storage layout than that of native bytes/string isn't always possible, which necessitates the current implementation.

    bytes data;
    bytes[] dataArray;

    struct BytesWrapper {
        bytes inner;
    }

    function f() public {
        data = "Asterix";
        delete data;
        dataArray.push("Milady");
        dataArray[0] = "Asterix";
        delete dataArray[0];
        bytes storage y = dataArray[0];
        // Error (7407): Type literal_string "Asterix" is not implicitly convertible to expected type bytes storage pointer.
        // y = "Asterix";
        // Error (9767): Built-in unary operator delete cannot be applied to type bytes storage pointer.
        // "delete y" is not valid, as assignments to local variables referencing storage objects 
        // can only be made from existing storage objects.
        // https://docs.soliditylang.org/en/latest/types.html#delete
        // delete y;
        BytesWrapper storage bw;
        assembly {
            bw.slot := y.slot
        }
        bw.inner = "Asterix";
        delete bw.inner;
    }

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • Ran forge fmt?
  • Ran forge test?

Pull requests with an incomplete checklist will be thrown out.

Add `set` and `delete_` functions to simplify handling of bytes and string storage references in `LibBytes` and `LibString`. These changes enable efficient and memory-safe operations when assigning or deleting storage references directly. Includes a helper for casting string storage to bytes storage.
Refactored the `set` function in `LibBytes` to handle unaligned storage writes and clean dirty bits, ensuring correctness. Introduced extensive testing, including native storage string handling, deletion, and random text scenarios, to verify the improved behavior. Added utility methods for more robust test coverage.
Introduce `setCalldata` functions in `LibString` and `LibBytes` to handle the assignment of calldata strings and bytes to storage references. Updated tests to cover the new functionality, ensuring proper handling and verification of calldata assignments.
@Vectorized
Copy link
Owner

O.O

@Vectorized
Copy link
Owner

Lemme marinate first.

May or may not merge.

Copy link

@Graysonbarton Graysonbarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants